Modern packaging + ruff + numpy 2 compat #187
Conversation
|
Sweet! Thanks @raphaelvallat, I was planning to include this in next release as well. I'll check it on my end in the next hour with a review. |
There was a problem hiding this comment.
@raphaelvallat sorry I know this is still WIP and you didn't request this, but I just glossed it over and here are small/quick things to add. All comments are trivially addressable and probably would've come up anyways during your checks so I hope they speed things up for you.
As far as Ruff formatting goes I think it will only end up changing 3 files when you run it. Currently the workflow breaks but it should pass once you run the formatter.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #187 +/- ##
==========================================
- Coverage 83.90% 77.20% -6.70%
==========================================
Files 24 13 -11
Lines 3498 2628 -870
Branches 0 321 +321
==========================================
- Hits 2935 2029 -906
Misses 563 563
- Partials 0 36 +36 ☔ View full report in Codecov by Sentry. |
|
Note that there's a CI failure on Python 3.12 because of a depreceated import of |
|
Ready for review @remrama ! There's one CI failure because of coverage that I think we can ignore |
raphaelvallat
changed the title
raphaelvallat
changed the title
There was a problem hiding this comment.
Looks great @raphaelvallat! Thanks a lot for doing this, I think it makes it easier to work on subsequent PRs.
.github/workflows/python_tests.yml
Outdated
| @@ -11,8 +11,8 @@ jobs: | |||
| strategy: | |||
| fail-fast: false | |||
| matrix: | |||
| platform: [ubuntu-latest, windows-latest] # macos-latest | |||
| python-version: ["3.9", "3.10", "3.11"] | |||
| platform: [ubuntu-latest, windows-latest] # macos-latest disabled because of lightgbm fail | |||
There was a problem hiding this comment.
Oh, I did not realize that YASA fails on mac... too bad, I wonder if there is a way around this in the future. Do you have any ideas on that? Just asking here but of course nothing to address for this PR 👍
There was a problem hiding this comment.
Problem solved — tests are now passing on macos as well 🚀
| @@ -26,7 +26,7 @@ classifiers = [ | |||
| "Programming Language :: Python :: 3.12", | |||
| ] | |||
| dynamic = ["version"] | |||
| requires-python = ">=3.8" | |||
| requires-python = ">=3.9" | |||
There was a problem hiding this comment.
Given the failure on python v3.12, should we change this to >=3.9, <3.12?
I think we should go forward as-is without >3.11 support, but given that Python currently supports versions up to 3.13, I would like to not fall that far behind. Maybe I can add a PR on lspopt about this? We could be patient about a fix and then come up with an alternate plan if need be.
There was a problem hiding this comment.
I would keep support for 3.12. I'll investigate the lspopt issue now
There was a problem hiding this comment.
Problem solved here as well. Just needed to add setuptools to the dependencies.
There was a problem hiding this comment.
Note that this still creates a DeprecationWarning in the tests. We should update lspopt at some point:
=============================== warnings summary ===============================
../../../../../opt/hostedtoolcache/Python/3.12.8/x64/lib/python3.12/site-packages/lspopt/data/__init__.py:11
/opt/hostedtoolcache/Python/3.12.8/x64/lib/python3.12/site-packages/lspopt/data/__init__.py:11: DeprecationWarning: pkg_resources is deprecated as an API. See https://setuptools.pypa.io/en/latest/pkg_resources.html
from pkg_resources import resource_filename
|
PR ready to be merged as soon as #127 is merged! |
This PR implements the following changes:
pyproject.toml. Source files are moved to thesrc/yasa/folder, while tests are now in thetestsfolder at the root directory